Improvement cql stmt generation #435
Conversation
CodeLieutenant
force-pushed
the
improvement-cql-stmt-generation
branch
2 times, most recently
from
776f81b to
905daff
Compare
There was a problem hiding this comment.
Amazing Refactor, @CodeLieutenant! I just had a couple of questions related to the implementation itself.
Btw, since you're reorganizing the whole code would be a good thing to drop some DocBlocks around things you think that's important for people to understand before jump into the code.
| sleep := time.Duration(0) | ||
|
|
||
| func NewPump(stopFlag *stop.Flag, logger *zap.Logger) chan time.Duration { | ||
| pump := make(chan time.Duration, 10000) | ||
| logger = logger.Named("Pump") | ||
| go func() { | ||
| logger.Debug("pump channel opened") | ||
| defer func() { | ||
| close(pump) | ||
| logger.Debug("pump channel closed") | ||
| }() | ||
| for !stopFlag.IsHardOrSoft() { | ||
| pump <- newHeartBeat() | ||
| } | ||
| }() | ||
| if rand.Int()%chance == 0 { | ||
| sleep = sleepDuration | ||
| } | ||
|
|
||
| return pump | ||
| pump <- sleep |
There was a problem hiding this comment.
Why it should sleep? Can you explain me exactly how it acts?
There was a problem hiding this comment.
Burst mechanism, makes a user of this, sleep after change is hit. practically if you make change = 10.
It will sleep approximatly one in 10. This is just to not overload the server
Statement generation currently does not work as intended, gemini generates values for primary and clustering keys, that are completply invalid. The are not just off by value, but also off by type, e.g. generates `text` for `timeuuid`. This is the first step in resolving this issue, by refactoring how, when and where statements are generated. Signed-off-by: Dusan Malusev <[email protected]>
1. Refactoring the Statement Generators into generators package 2. Each table gets it's own random number for validation and generation 3. Warmup consolidated into Job, runs with its own validator and statement generator -> propably can be removed, as Warmup is just a mutation without DDL and deletes Future Plans 1. Remove stopFlag as context.Context can do the same, hard kill is to CANCEL the Global Parent, and soft kill is cancelation of the current context 2. Generators Refactoring Signed-off-by: Dusan Malusev <[email protected]>
context.Context Go has built it mehanizm to stop the execution of the long running process, like goroutines and network requests, and many libraries using inside gemini support it, lets just make a heavy usage of it, cause we can. Signed-off-by: Dusan Malusev <[email protected]>
CodeLieutenant
force-pushed
the
improvement-cql-stmt-generation
branch
from
bcf3e1f to
185729d
Compare
Signed-off-by: Dusan Malusev <[email protected]>
…6 threads on small cluster Signed-off-by: Dusan Malusev <[email protected]>
pkg/jobs/warmup.go
Outdated
There was a problem hiding this comment.
why this is removed in this commmit context ?
pkg/jobs/jobs.go
Outdated
There was a problem hiding this comment.
the main thing missing from this commit description is, what's wrong with the stopFlags
There was a problem hiding this comment.
Multiple issues, it's too complex, makes gemini stuck sometimes when SIGTERM or SIGINT is sent and it's never tested, context.Context is in stdlib, fully tested and everybody in go ecosystem knows how to use it. TLDR makes gemini code shorted and easier to maintain without hiccups
There was a problem hiding this comment.
- too complex is a bit vague argument.
- "everybody in go ecosystem knows how to use" - maybe I'm not yet that familiar of go ecosystem evaluate that statement.
we need to write all of it down, i.e. what the shortcoming of it, vs. what is the advantages of context.Context
preferably in the commit message / PR description
Signed-off-by: Dusan Malusev <[email protected]>
statement generator -> propably can be removed, as Warmup is just a
mutation without DDL and deletes
Future Plans
to CANCEL the Global Parent, and soft kill is cancelation of the
current context